Skip to content

Adding ADR-0009 - #1861

Closed
bendk wants to merge 1 commit into
mozilla:mainfrom
bendk:adr-0009
Closed

bendk wants to merge 1 commit into
mozilla:mainfrom
bendk:adr-0009

Conversation

@bendk

@bendk bendk commented Nov 22, 2023

Copy link
Copy Markdown
Contributor

Use handle map handles to pass objects across the FFI,

@bendk
bendk requested a review from a team as a code owner November 22, 2023 18:57
@bendk
bendk requested review from badboy and removed request for a team November 22, 2023 18:58
@bendk

bendk commented Nov 22, 2023

Copy link
Copy Markdown
Contributor Author

I'll state my preference up-front: I think we should be using handles.

I don't like 1 because it's too hard to debug errors. Whenever I want to add new functionality that involves passing an object, I almost always mess it up at least once and currently this results in an impossible to understand stack trace or a crash with no message at all. When I was trying to switch things over to using a handle map in #1808, I definitely had bugs like this, but getting the nice error message was much nicer.

I don't like 4 because I don't want to add the dependency for all users.

I slightly prefer 3 over 2, but I'd be fine with either of those.

@bendk

bendk commented Nov 22, 2023

Copy link
Copy Markdown
Contributor Author

I think the changes in #1823 and #1826 are good regardless of this decision, so the ADR doesn't mention them. Please speak up if you think some of those should be part of the ADR, or another one.

This ADR might guide how some of that code looks, for example naming the variant FfiType::Handle makes more sense if we choose option 2 or 3.

@badboy

badboy commented Nov 23, 2023

Copy link
Copy Markdown
Member

Rendered

@bendk bendk mentioned this pull request Dec 1, 2023
This revisits the decision in `ADR-0005` and explores using going back to handle maps to pass objects across the FFI.
@bendk

bendk commented Dec 6, 2023

Copy link
Copy Markdown
Contributor Author

Just pushed some small changes to the text and added a document that tries to describe how the handlemaps would work in detail: https://github.com/mozilla/uniffi-rs/blob/6307aa0561312472557c8770da1042a7d74a24ad/docs/handles.md

@bendk

bendk commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

I'm going to close this one. We never came to agreement and I think that's fine. The current status quo is:

  • We have a Handle type, which is an opaque 64-bit value
  • Rust handles are essentially leaked Arc references. This is the fastest option and we can live with the "hard to debug errors" part. It's a bit awkward for trait interfaces, which need to an extra Arc to trap them but that's okay.
  • Foreign handles currently use a Handle map, but bindings could choose to switch to leaking/casting pointers if they wanted to. We currently ensure the handles always have the lowest bit set, which is needed to make trait interfaces efficient. If foreign sides switched to casted pointers, they would just need to do some bit flipping to ensure the same thing.

All of this seems okay and doesn't require changing in the short term. Also, my current thinking is that we don't need to design a single official way of doing things. I'd rather follow the model used for uniffi-bindgen-kotlin-jni and allow bindings to customize things a bit if they want.

@bendk bendk closed this Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants